Fix npm v1 lock losing a hosted patch on takeover (#659) - #660
Mikola Lysenko (mikolalysenko) merged 3 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A hosted npm pin on a lockfileVersion 1 lock is lost when scan/get --mode vendored takes it over: the upstream entry is restored before the vendored backend refuses the v1 lock (#659). Assisted-by: Claude Code:claude-opus-5-5
On an npm 6 (lockfileVersion 1) project, scan/get --mode vendored restored a hosted patch's upstream entry before the vendored backend refused the v1 lock, so the project silently went back to unpatched. The takeover now runs the npm package-lock lock gate first, as it already did for Bun, vlt and yarn berry: the purl is refused with vendor_lockfile_version_unsupported and stays hosted. The vendored dry-run preview lists such purls as would_refuse. Fixes #659 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dcde03b. Configure here.
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
|
[burn-down agent] Ready for review at head
Slack announcement: pending (Slack send tool unavailable in this run; next run retries). Generated by Claude Code |
|
Codex review of The new preflight uses the backend's own lock selection, JSON parser and version/shape gate, including shrinkwrap precedence. It refuses unsupported locks before scan/get or manifest-based vendor removes the hosted pin. Supported takeover and other package-manager routing remain intact. Validation:
The separate manifestless Fresh CI is clear: 485 successful checks, 7 skipped; 13 successful workflows and 1 skipped. Bugbot is clear on this exact commit, with no unresolved threads or outstanding actionable feedback. GitHub's normal human approval requirement remains before merge. |
3 participants
LLM Description written by Claude Code:claude-opus-5-5
Fixes #659
Summary
On an npm 6 project (lockfileVersion 1
package-lock.json/npm-shrinkwrap.json),scan --mode vendored/get --mode vendoredover a hosted patch un-hosted it and then refused to vendor it, so the project silently went back to unpatched. The refusal now happens before anything is restored: the run still fails withvendor_lockfile_version_unsupported, but the hosted pin and.npmrcstay byte-identical, so the package stays hosted-patched. The dry run previews the refusal instead of "Would download and vendor 1 patch".Root cause
The hosted→vendored takeover (
commands/vendor.rs) restores the upstream registry entry before the npm package-lock backend runs its lockfile-version gate (vendor/npm_lock.rslock_version_gate). The Bun, vlt, yarn berry and gem backends already raise their lock-text refusals ahead of the restore; npm package-lock did not.Change
npm_lock_vendor_preflight(exported fromvendor), the backend's own step-2 gate (parse + v2/v3 check over the lockselect_lockfilepicks), returning the backend's exact(code, detail). It only answers for package-lock-flavored projects.commands/vendor.rs): the project-level takeover preflight that was berry-only now also consults the npm package-lock preflight beforerestore_upstream, dry and wet.scan/vendor_flow.rs):scan/get --mode vendored --dry-runlists npm purls of such a project aswould_refusewith that code (consistent with the existing Bun/vlt preview rows; does not change the dry-run exit code, per the preview contract).Tests
New
crates/socket-patch-cli/tests/in_process_vendor_npm_v1_takeover.rs(hermetic: wiremock API and npm registry; the hosted state is produced by a realscan --mode hosted):scan --mode vendored, v1package-lock.json(+ dry-run preview)scan_vendored_over_hosted_v1_package_lock_keeps_the_hosted_pinget --mode vendoredget_vendored_over_hosted_v1_package_lock_keeps_the_hosted_pinvendor_takeover_reverted_redirect, pin gone)npm-shrinkwrap.jsonscan_vendored_over_hosted_v1_shrinkwrap_keeps_the_hosted_pinscan_vendored_over_hosted_v2_package_lock_still_takes_overCore unit tests:
npm_lock::tests::preflight_matches_the_backend_v1_refusal(same code and detail as the backend) andpreflight_passes_v3_and_other_flavors.Local runs:
cargo clippy --workspace --all-features -- -D warnings: clean.cargo fmt --all -- --check: the files this PR touches are clean.mainalready has unrelated rustfmt drift, and CI doesn't run fmt.cargo test --workspace --all-features: all pass except 12 tests that rely on chmod-based write failures (e.g.covgap_commands_vendor::vendor_state_write_failure_reports_failed_event,repair_invariants::repair_cleanup_failure_…,pypi_poetry::wire_write_failure_…). Those fail because this sandbox runs as root, which bypasses the permission denial. They don't touch this diff; CI runs as non-root.e2e_vendor_npm_build --include-ignoredwithSOCKET_PATCH_NPM_E2E_REQUIRED=1(npm 10.9.4): 17/17 pass.e2e_redirect_npm_build: 13/15 pass. The 2 failures arerollbackfailing to reachhttps://registry.npmjs.orgfrom this sandbox's HTTP client (error sending request). That code path isn't touched here; CI covers it.Bugbot: reviewed dcde03b, no issues.
Note
Medium Risk
Changes hosted→vendored takeover ordering for npm projects and lockfile mutation timing, though behavior is stricter (fail earlier) and covered by new integration tests.
Overview
Fixes #659, where
scan/get --mode vendored(andvendor) over a hosted npm 6 pin could restore the upstream lock entry first, then fail withvendor_lockfile_version_unsupportedbecause vendoring only supports lockfileVersion 2/3—leaving the project unpatched with the hosted wiring already removed.The npm package-lock backend now exposes
npm_lock_vendor_preflight, mirroring Bun/vlt/Yarn berry: it runs the same v2/v3 (and parse) gate before any takeover restore. Refused runs still exit with the same error code, butpackage-lock.json/npm-shrinkwrap.jsonand.npmrcstay unchanged, so the package remains hosted-patched. Vendored dry-run JSON now marks all npm purls in such projects aswould_refusewith that code, consistent with other preflight previews.CLI contract and npm compatibility docs describe the new ordering; hermetic CLI tests cover v1 package-lock, shrinkwrap, dry-run preview, and a v2 control that still completes takeover.
Reviewed by Cursor Bugbot for commit dcde03b. Configure here.
Generated by Claude Code